Improved sim handling of $display/$print
#3963
Conversation
This allows tools like SBY to capture the $display output independent from anything else sim might log. Additionally it provides source and hierarchy locations for everything printed.
|
That's what I get for not trying a local build after rebasing... |
These are useful for formal verification with SBY where they can be used to display solver chosen `rand const reg` signals and signals derived from those. The previous error message for non-constant initial $display statements is downgraded to a log message. Constant initial $display statements will be shown both during elaboration and become part of the RTLIL so that the `sim` output is complete.
This keeps %m as a format part of the RTLIL format syntax, as %m is supposed to be the hierarchical name of the current instance, which is not statically known for an RTLIL module. To allow sim to output the correct hierarchical name and the correct simulation time, the `Fmt::render` function is extended to take both as argument. When `render` is called in a constant context, this keeps essentially the previous behavior and uses the module name as fallback, as the hierarchy is not known at that point.
|
FYI: In anticipation of this to be merged, I made a PR in sv-tests to invoke the simulation pass in Yosys: #5498. Merging of the current PR will then better reflect on the capabilites of Yosys in sv-tests, and make more working tests available for further development of Yosys. Yes, I have tested this 😄 The only issue I can see with the current PR right off the bat is that a |
| @@ -734,7 +761,9 @@ std::string Fmt::render() const | |||
| buf = part.sig.as_const().decode_string(); | |||
| } else if (part.type == FmtPart::TIME) { | |||
| // We only render() during initial, so time is always zero. | |||
There was a problem hiding this comment.
This comment no longer applies.
| arg.type = VerilogFmtArg::HIERNAME; | ||
| args.push_back(arg); | ||
| fmt.str += '%'; | ||
| if (part.plus) |
There was a problem hiding this comment.
Are these modifiers actually valid for %m?
There was a problem hiding this comment.
AFAICT the spec is not clear on this and IIRC there was at least some inconsistency between other implementations. In places where the spec is unclear I think it would make sense to support whatever other commonly used open-source tools like iverilog support as long as that's a reasonable interpretation of the spec (although I don't remember what exactly iverilog does here and in general I'd also be fine with not supporting any modifiers for %m).
There was a problem hiding this comment.
In places where the spec is unclear I think it would make sense to support whatever other commonly used open-source tools like iverilog support as long as that's a reasonable interpretation of the spec (although I don't remember what exactly iverilog does here and in general I'd also be fine with not supporting any modifiers for %m).
I agree with this stance. I just want to make sure that whichever modifiers we implement here actually match other tools, if we do support them.
This contains 3 changes that improve support for
$displaycalls /$printcells in the context of simulating formal verification counterexample and cover traces usingsim(used by SBY withfst onorvcd_sim on, will become the default in the future):0e92cc1
Adds printed output to the JSON summary that SBY parses. This includes source and hierarchy information of the
$printcell which might be useful even for othersimuses. I still have to add functionality to SBY to actually do anything with this data.8c89b46
Prior to this,
$displaycalls inside initial statements were not emitted as$printcells and generated an error if they were not using constant values. During FV it's common to have constant-but-solver-determined values (rand const regor$anyconst) and outputting those in a structured way using $display in an initial block is something I very much want to do. This change still produces an elaboration time output when the values are constant but downgrades the error otherwise to an info message for calls that output the formatted string. In both cases it emits a $print cell so that the output during simulation is complete. It remains an error to use non-constant inputs for formatting functions that return the formatted string.1dbd3cc
The current handling of
%mstatically inserts the module name, while the SV spec says that the hierarchical name should be output. Printing the hierarchical name is very useful when inserting debug logging to a module instantiated several times in the design. Since the hierarchical name is not known at the time the format string is processed, this requires actually keeping it as a format arg. To support it duringsim, I added an argument torender()sosimcan supply the correct hierarchical name. For the other elaboration time calls torender()I keep using the module name as the hierarchical name isn't statically known in that context.Additionally
render()assumed that the time is always0as it was only used for constant elaboration time calls before #3915, but since this isn't the case anymore I added another argument so thatsimcan supply the correct simulation time.What is still missing, and the main reason this is a draft PR, is hierarchical
%msupport for CXXRTL. As I haven't really used CXXRTL so far I'm not sure what needs to be done and wanted to get some feedback on the general approach before I figure out what to do there. I'd also want to add some tests for the added functionality.